Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor(compilation): Add callback to factorizeTask and make process dependencies happen after all dependencies get factorized #4835

Merged
merged 1 commit into from
Jan 17, 2024
Merged

refactor(compilation): Add callback to factorizeTask and make process dependencies happen after all dependencies get factorized #4835

merged 1 commit into from
Jan 17, 2024

Conversation

JSerFeng
Copy link
Collaborator

@JSerFeng JSerFeng commented Nov 30, 2023 •
edited
Loading

Summary

Add callback parameter to handle_module_creation

In Webpack, processDependencies task finishes after all the dependencies actually factorized. I use a oneshot channel to get informed

Test Plan

Require Documentation?

  • No
  • Yes, the corresponding rspack-website PR is __

@JSerFeng
Copy link
Collaborator Author

JSerFeng commented Nov 30, 2023 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @JSerFeng and the rest of your teammates on Graphite Graphite

@github-actions github-actions bot added the team The issue/pr is created by the member of Rspack. label Nov 30, 2023
This was referenced Nov 30, 2023
Merged
Merged
@JSerFeng JSerFeng force-pushed the 11-30-refactor_compilation_process_dependencies_should_happen_after_all_dependencies_get_factorized branch from 80f8756 to c1ced85 Compare December 1, 2023 07:05
@IWANABETHATGUY
Copy link
Contributor

!bench

@rspack-bot
Copy link

rspack-bot commented Dec 1, 2023 •
edited
Loading

📝 Benchmark detail: Open

name base current %
10000_development-mode + exec 1.48 s ± 28 ms 1.5 s ± 21 ms +1.49%
10000_development-mode_hmr + exec 855 ms ± 2.4 ms 853 ms ± 4.9 ms -0.19%
10000_production-mode + exec 3.66 s ± 25 ms 3.83 s ± 74 ms +4.68%
threejs_development-mode_10x + exec 2.01 s ± 25 ms 2.03 s ± 23 ms +1.26%
threejs_development-mode_10x_hmr + exec 1.25 s ± 11 ms 1.25 s ± 13 ms +0.61%
threejs_production-mode_10x + exec 6.05 s ± 40 ms 6.03 s ± 25 ms -0.24%

@JSerFeng JSerFeng changed the title refactor(compilation): process dependencies should happen after all dependencies get factorized refactor(compilation): Add callback to factorizeTask and make process dependencies happen after all dependencies get factorized Dec 1, 2023
@JSerFeng
Copy link
Collaborator Author

JSerFeng commented Dec 1, 2023

!bench

@rspack-bot
Copy link

rspack-bot commented Dec 1, 2023 •
edited
Loading

📝 Benchmark detail: Open

name base current %
10000_development-mode + exec 1.48 s ± 28 ms 1.51 s ± 19 ms +1.86%
10000_development-mode_hmr + exec 855 ms ± 2.4 ms 853 ms ± 8.6 ms -0.22%
10000_production-mode + exec 3.66 s ± 25 ms 3.84 s ± 36 ms +5.01%
threejs_development-mode_10x + exec 2.01 s ± 25 ms 2.03 s ± 17 ms +0.88%
threejs_development-mode_10x_hmr + exec 1.25 s ± 11 ms 1.25 s ± 6.5 ms +0.55%
threejs_production-mode_10x + exec 6.05 s ± 40 ms 6.05 s ± 29 ms +0.05%

Threshold exceeded: ["10000_production-mode + exec"]

IWANABETHATGUY
IWANABETHATGUY previously approved these changes Dec 1, 2023
View reviewed changes
@JSerFeng JSerFeng force-pushed the 11-30-refactor_compilation_process_dependencies_should_happen_after_all_dependencies_get_factorized branch from c1ced85 to 9f2d540 Compare December 2, 2023 09:35
IWANABETHATGUY
IWANABETHATGUY previously approved these changes Dec 4, 2023
View reviewed changes
@JSerFeng JSerFeng force-pushed the 11-30-refactor_compilation_process_dependencies_should_happen_after_all_dependencies_get_factorized branch from 9f2d540 to 47d1afa Compare December 6, 2023 07:04
@JSerFeng JSerFeng force-pushed the 11-30-refactor_compilation_process_dependencies_should_happen_after_all_dependencies_get_factorized branch 3 times, most recently from 50d2142 to f944513 Compare December 16, 2023 11:01
@JSerFeng JSerFeng force-pushed the 11-30-refactor_compilation_process_dependencies_should_happen_after_all_dependencies_get_factorized branch 2 times, most recently from e1c4553 to caa3be2 Compare December 20, 2023 06:37
@JSerFeng JSerFeng mentioned this pull request Dec 20, 2023
Merged
2 tasks
@JSerFeng JSerFeng force-pushed the 11-30-refactor_compilation_process_dependencies_should_happen_after_all_dependencies_get_factorized branch 3 times, most recently from 1777769 to 90f1652 Compare December 26, 2023 18:03
@JSerFeng JSerFeng force-pushed the 11-30-refactor_compilation_process_dependencies_should_happen_after_all_dependencies_get_factorized branch from 90f1652 to 347b90e Compare January 4, 2024 07:39
@JSerFeng JSerFeng mentioned this pull request Jan 4, 2024
Merged
2 tasks
@JSerFeng JSerFeng force-pushed the 11-30-refactor_compilation_process_dependencies_should_happen_after_all_dependencies_get_factorized branch 2 times, most recently from 25d8841 to f85604d Compare January 10, 2024 13:55
@JSerFeng JSerFeng marked this pull request as draft January 12, 2024 13:21
@JSerFeng JSerFeng force-pushed the 11-30-refactor_compilation_process_dependencies_should_happen_after_all_dependencies_get_factorized branch from f85604d to 21b6f6f Compare January 12, 2024 13:57
@JSerFeng JSerFeng marked this pull request as ready for review January 16, 2024 06:43
@ahabhgk
Copy link
Collaborator

ahabhgk commented Jan 16, 2024

!bench

@rspack-bot
Copy link

rspack-bot commented Jan 16, 2024 •
edited
Loading

📝 Benchmark detail: Open

name base current %
10000_development-mode + exec 1.65 s ± 30 ms 1.69 s ± 37 ms +2.48%
10000_development-mode_hmr + exec 1.01 s ± 12 ms 1 s ± 11 ms -0.91%
10000_production-mode + exec 2.91 s ± 37 ms 2.95 s ± 52 ms +1.21%
threejs_development-mode_10x + exec 2.01 s ± 32 ms 2 s ± 21 ms -0.64%
threejs_development-mode_10x_hmr + exec 1.33 s ± 4.7 ms 1.32 s ± 13 ms -0.71%
threejs_production-mode_10x + exec 6.1 s ± 73 ms 5.97 s ± 51 ms -2.24%

ahabhgk
ahabhgk previously approved these changes Jan 16, 2024
View reviewed changes
@JSerFeng JSerFeng force-pushed the 11-30-refactor_compilation_process_dependencies_should_happen_after_all_dependencies_get_factorized branch from 21b6f6f to a947386 Compare January 17, 2024 03:06
@JSerFeng JSerFeng merged commit 5031972 into main Jan 17, 2024
17 checks passed
@JSerFeng JSerFeng deleted the 11-30-refactor_compilation_process_dependencies_should_happen_after_all_dependencies_get_factorized branch January 17, 2024 04:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahabhgk ahabhgk ahabhgk approved these changes

@IWANABETHATGUY IWANABETHATGUY IWANABETHATGUY left review comments

@ScriptedAlchemy ScriptedAlchemy Awaiting requested review from ScriptedAlchemy

@jerrykingxyz jerrykingxyz Awaiting requested review from jerrykingxyz

Assignees

@JSerFeng JSerFeng

Labels
team The issue/pr is created by the member of Rspack.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@JSerFeng @IWANABETHATGUY @rspack-bot @ahabhgk